Skip to content

TNL-930: Turn off Student Notes when Harvard Annotation Tool is enabled. - #6340

Merged
tymofij merged 1 commit into
feature/edxnotesfrom
anton/harvard-tool-student-notes
Dec 30, 2014
Merged

TNL-930: Turn off Student Notes when Harvard Annotation Tool is enabled.#6340
tymofij merged 1 commit into
feature/edxnotesfrom
anton/harvard-tool-student-notes

Conversation

@polesye

@polesye polesye commented Dec 22, 2014

Copy link
Copy Markdown
Contributor

Disables Student Notes if one of textannotation, imageannotation, videoannotation modules exist in the list of Advanced Modules.

There is still possibility to break Student Notes and Harvard Annotation Tool:

  • add one of textannotation, imageannotation, videoannotation to the Advanced Modules list;
  • add one of textannotation, imageannotation, videoannotation to the unit;
  • remove it form the Advanced Modules list => You can enable Student Notes now. If you have both modules in the sequential, it breaks one of these modules.

I think we should write a good note about this situation.

@tymofij , @olmar, @jmclaus, @srpearce please review.

sandbox: http://studio.polesye.m.sandbox.edx.org/

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@polesye Instead of Note: Harvard Annotation... I would use Warning: Harvard Annotation..., Note can be confusing in the Notes context and doesn't emphasize enough that things won't work if Harvard Annotation Tool is on.

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

According to the PMs, Harvard is the only school that has access to the Harvard annotation tool. I'm concerned that people who see it mentioned here will start asking about the tool and wanting to use it - so I think this message should go directly from PMs to Harvard instead of appearing to all of our worldwide users in Studio.

Also, can the display name be "Enable Student Notes," since that's what we've been calling this feature?

Assuming we can change the display name and we don't want to mention any other annotation tools in the UI, I'd revise this text to just the following:

Enable Student Notes
Enter true or false. If true, students can use the Student Notes feature.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Enable Student Notes
Enter true or false. If true, students can use the Student Notes feature.

Done. Thanks.

@polesye

polesye commented Dec 22, 2014

Copy link
Copy Markdown
Contributor Author

@srpearce sandbox is created ^^.

Comment thread lms/djangoapps/edxnotes/tests.py Outdated

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Tests that get_html is not wrapped...

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Tests that get_html is not wrapped...

Thanks, fixed.

@polesye

polesye commented Dec 23, 2014

Copy link
Copy Markdown
Contributor Author

@jmclaus , @olmar , @tymofij , @srpearce all comments are addressed. Please continue review.

@jmclaus

jmclaus commented Dec 23, 2014

Copy link
Copy Markdown

@polesye 👍

1 similar comment
@olmar

olmar commented Dec 23, 2014

Copy link
Copy Markdown
Contributor

@polesye 👍

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

display name: Enable Student Notes in Course
help: Enter true or false. If true, students can use the Student Notes feature.

I hesitate to use something like "Make Student Notes Visible" because that could raise questions about whether students can make notes even if this value is set to false, but can only see them if this is set to true. "Enable Student Notes in Course" indicates more clearly that a student can't perform any action with Notes at all unless this value is set to true.

If "Enable Student Notes in Course" is too long, I'd suggest changing the display name for edxnotes to "Enable Student Notes in Platform" (since fewer people will see this) and changing the display name for edxnotes_visibility to "Enable Student Notes".

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

display name: Enable Student Notes in Course
help: Enter true or false. If true, students can use the Student Notes feature.

I hesitate to use something like "Make Student Notes Visible" because that could raise questions about whether students can make notes even if this value is set to false, but can only see them if this is set to true. "Enable Student Notes in Course" indicates more clearly that a student can't perform any action with Notes at all unless this value is set to true.

If "Enable Student Notes in Course" is too long, I'd suggest changing the display name for edxnotes to "Enable Student Notes in Platform" (since fewer people will see this) and changing the display name for edxnotes_visibility to "Enable Student Notes".

This field is not user visible. So, I just removed display name, and updated help string so Indicates current state of visibility of the Student Notes. This can be changed by a student to show/hide all notes in Platform. For more information: https://openedx.atlassian.net/browse/TNL-661
@srpearce what do you think?

@polesye polesye mentioned this pull request Dec 24, 2014
@polesye
polesye force-pushed the anton/harvard-tool-student-notes branch from 4296776 to 487c799 Compare December 24, 2014 15:44

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'd change it to:

Student Notes Visibility
Indicates whether Student Notes are visible in the course. Students can also show or hide their notes in the courseware.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'd change it to:

Student Notes Visibility
Indicates whether Student Notes are visible in the course. Students can also show or hide their notes in the courseware.

changed.

@polesye
polesye force-pushed the anton/harvard-tool-student-notes branch from 487c799 to 30de79a Compare December 25, 2014 12:52
@tymofij
tymofij force-pushed the anton/harvard-tool-student-notes branch from 30de79a to 53772e8 Compare December 30, 2014 12:19
@tymofij
tymofij force-pushed the anton/harvard-tool-student-notes branch from 53772e8 to 8c58025 Compare December 30, 2014 12:39
@tymofij

tymofij commented Dec 30, 2014

Copy link
Copy Markdown
Contributor

👍 once tests pass

tymofij added a commit that referenced this pull request Dec 30, 2014
TNL-930: Turn off Student Notes when Harvard Annotation Tool is enabled.
@tymofij
tymofij merged commit 871ecd7 into feature/edxnotes Dec 30, 2014
@benpatterson
benpatterson deleted the anton/harvard-tool-student-notes branch August 2, 2016 13:25
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants